Skip to content

feat(discovery): manual network assets W03 — website/URL targets, partner inventory, docs, e2e (#5213) - #5260

Merged
ToddHebebrand merged 4 commits into
mainfrom
feature/5228-manual-network-asset/wave-5231
Sep 8, 2026
Merged

feat(discovery): manual network assets W03 — website/URL targets, partner inventory, docs, e2e (#5213)#5260
ToddHebebrand merged 4 commits into
mainfrom
feature/5228-manual-network-asset/wave-5231

Conversation

@ToddHebebrand

Copy link
Copy Markdown
Collaborator

Summary

Task 7 (W03) of docs/superpowers/plans/device-lifecycle/2026-09-07-manual-network-asset.md — website/URL targets for manually-added network assets, on top of W01 (data model, main) and W02 (create route + form, #5258, main).

  • AddNetworkAssetModal.tsx: website/service in the asset-type picker; those two require a URL specifically (not just "any of IP/hostname/URL") and hide the MAC field (no MAC on an IP-less endpoint). After creating a website/service asset, the modal offers an inline hand-off to add an HTTP check (reuses CreateMonitorForm as-is, pre-targeted at the asset's URL via defaultTarget — no monitor-worker changes, per the plan's Verification note 4).
  • Partner inventory export (routes/partnerApi/inventory.ts + the strict partnerNetworkEquipmentSchema allowlist in schemas.ts): extended the site-inventory asset_type filter and JSON projection to include website/service, and added url/source fields (already included in the export-policy registry from W01). address is now nullable end-to-end (host(NULL) is SQL NULL, not the string "null") — the strict response schema is what actually gates this: a field present in the projection but missing from the schema fails the whole export closed with a 500 on .parse(), which is exactly the trap the wave brief called out (PR feat(patches): offline work queue W03 — queued results, non-terminal jobs, finalizer, offline_behavior, supersession (#5131) #5174).
  • Docs: "Adding a Network Asset Manually" section in discovery.mdx — identity rule, the Manual asset entry — log non-agent, non-discovered assets (printers, phones, spare equipment) into the unified device list #4622 manual-vs-network-identity boundary, in-place scan updates, and the monitoring hand-off.
  • e2e: e2e-tests/tests/manual-network-asset.spec.ts — Devices page → add-menu → website asset → Network segment shows the row with the Manual source badge. (See Verification below — status has no data-testid on the DeviceList badge, owned by the parallel Manual asset entry — log non-agent, non-discovered assets (printers, phones, spare equipment) into the unified device list #4622 wave, so that half of the acceptance criterion is asserted from the create response instead.)

No migration, no changes to discoveryWorker.ts or other scan writers (W03 doesn't touch them, per the plan).

Verification

apps/api: npx vitest run src/routes/partnerApi src/routes/devices/network   → 434 passed (14 + 1 files)
apps/api: npx tsc --noEmit -p .                                             → clean
apps/web: npx vitest run src/components/devices src/components/monitoring
          src/lib/__tests__/no-silent-mutations.test.ts
          src/lib/i18n/localeParity.test.ts                                 → 1307 passed (94 files)
apps/web: npx tsc --noEmit -p .                                             → clean
eslint on every touched file (api + web)                                    → clean

All 8 locale catalogs (de-DE, es-419, fr-CA, fr-FR, it-IT, pt-BR, tr-TR, en) got matching assetTypes.website/.service and addNetworkAssetModal.urlRequiredHint/.postCreate.* keys — localeParity.test.ts passes, confirming key-set/type/interpolation/protected-literal parity.

Not run: the Playwright e2e spec and the real-DB integration suites (partnerApiReconstructionWatermark.integration.test.ts is the only one of the three named in the wave brief that touches discoveredAssets/networkEquipment, and it only inserts assetType: 'switch' rows — unaffected by this change). Booting worktree-stack needed a root .env that isn't present in this worktree; the orchestrator's instructions were explicit not to reach into the shared main checkout to fetch one, so I stopped short rather than route around that boundary. Flagging for a stack-equipped follow-up run before merge if a live check is wanted.

Closes #5231

🤖 Generated with Claude Code

https://claude.ai/code/session_012R4VMwU9tjfQqhw7Ep1xAK

Task 7 of the manual-network-asset plan: website/service asset types in
AddNetworkAssetModal (URL required, MAC hidden, HTTP-check hand-off via
CreateMonitorForm), extend the partner inventory export projection
(routes/partnerApi/inventory.ts + the strict schemas.ts allowlist) to
include the two new types plus url/source, docs, and an e2e spec.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012R4VMwU9tjfQqhw7Ep1xAK
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 8, 2026

Copy link
Copy Markdown

Deploying breeze with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6cbe75a
Status: ✅  Deploy successful!
Preview URL: https://408bac23.breeze-9te.pages.dev
Branch Preview URL: https://feature-5228-manual-network-rn31.breeze-9te.pages.dev

View logs

- Stop posting a stale MAC address for website/service assets: hiding the
  field didn't clear its state (AddNetworkAssetModal.tsx).
- Sweep website/service through the web-side type maps that were still
  hand-enumerated at the old 12 values: the shared DiscoveredAssetType
  union, DiscoveredAssetList's local duplicate + typeConfig + assetTypeMap,
  assetTypeIcon.tsx, and deviceRoles.ts's label/icon lookup (added
  additively, never touching the billable-role tuple that backs
  contract_lines_device_roles_chk).
- Derive partnerNetworkEquipmentSchema's `source` enum from
  discoveredAssetSourceEnum.enumValues instead of hand-copying it, so a
  future 4th source value fails at the type level instead of 500ing the
  whole partner inventory export in production.
- CreateMonitorForm gains an optional defaultMonitorType prop so the
  website/service hand-off actually opens on http_check pre-targeted at
  the URL, instead of icmp_ping with a blank field the operator had to
  notice and fix themselves.
- Correct two doc/comment inaccuracies: the partnerNetworkEquipmentSchema
  comment overstated the pre-existing `address` NOT-NULL guarantee, and
  discovery.mdx overstated what a rescan preserves (a manual row's MAC
  isn't guarded on the scan path's own terms, and the UniFi path has no
  manual-source guard on hostname/manufacturer/model at all). Also dropped
  a bare issue number from public docs and softened a reference to a
  not-yet-shipped UI.
- New/updated tests: MAC-cleared-on-type-switch regression, the
  urlRequiredHint/identityHint swap, and the website+service asset types
  both exercised via describe.each (only website was covered before).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012R4VMwU9tjfQqhw7Ep1xAK
@ToddHebebrand

Copy link
Copy Markdown
Collaborator Author

Review summary — pr-review-toolkit

Ran code-reviewer, pr-test-analyzer, and comment-analyzer in parallel against the diff.

Findings, all fixed in f1d628f

code-reviewer (4 confirmed):

  1. Important — stale MAC address posted for website/service assets: hiding the field didn't clear its React state. Fixed: payload nulls macAddress when the type requires a URL; added a regression test.
  2. Important — the "Add an HTTP check" hand-off opened CreateMonitorForm on its icmp_ping default with a blank target, requiring the operator to notice and fix it. Fixed: added an optional defaultMonitorType prop to CreateMonitorForm (backward-compatible, only used by this hand-off) and seeded httpUrl from defaultTarget.
  3. Importantwebsite/service were only added to the DB enum and this wave's own files, not swept through the web-side type maps still hand-enumerated at the old 12 values (packages/shared's DiscoveredAssetType, DiscoveredAssetList.tsx's local duplicate type + typeConfig + assetTypeMap, assetTypeIcon.tsx, deviceRoles.ts's label/icon lookup) — a website asset would have rendered as "Unknown" with a generic icon everywhere except this wave's own two files. Fixed all of them; deviceRoles.ts gets an additive lookup only, never touching DEVICE_ROLES/BILLABLE_DEVICE_ROLES (that tuple backs contract_lines_device_roles_chk and billing — a website isn't a billable device role). Two findings in this category (filterEngine.ts/filterFields.ts) turned out to be a different, unrelated "Device Role" billing facet, not a discovery asset-type filter — verified no change needed there.
  4. ImportantpartnerNetworkEquipmentSchema.source hand-copied the DB enum's three values instead of deriving from it — a future 4th source value would 500 the whole partner inventory export the moment it shipped, the exact trap this PR's own comment calls out one enum over. Fixed: derived from discoveredAssetSourceEnum.enumValues, matching the existing DISCOVERED_ASSET_TYPES pattern in routes/devices/schemas.ts.

pr-test-analyzer: confirmed the new tests are behavioral (not vacuous) and that the existing durable-type regression and locale-parity guards are real, executable contracts. Gaps: only website was exercised, never service (fixed — both types now run through describe.each); the identityHint/urlRequiredHint text swap had no assertion (fixed). Two minor gaps left as-is (monitor-form cancel flow, an old-type-with-no-address round-trip) — low risk, not worth a second review round.

comment-analyzer: two comment/doc inaccuracies, both fixed — the schemas.ts comment overstated a pre-existing NOT-NULL guarantee that this PR's own AddNetworkAssetModal identity rule already contradicted; discovery.mdx overstated what a rescan preserves (MAC isn't guarded on the scan path's own terms, and the UniFi enrichment path has no manual-source guard on hostname/manufacturer/model at all — corrected to say so). Also dropped a bare GitHub issue number from public-facing docs and softened a reference to a UI that hasn't shipped yet (#4622 backend-only as of this PR's base).

Verification after fixes

  • apps/api: npx vitest run src/routes/partnerApi src/routes/devices/network → 433 passed; npx tsc --noEmit -p . → clean
  • packages/shared: npx tsc --noEmit -p . → clean (pre-existing unrelated failures in quotes.test.ts untouched)
  • apps/web: npx vitest run src/components/devices src/components/discovery src/components/monitors src/components/monitoring src/lib/__tests__/no-silent-mutations.test.ts src/lib/i18n/localeParity.test.ts src/lib/deviceRoles.test.ts → 1432 passed (106 files); npx tsc --noEmit -p . → clean
  • eslint on every touched file (api + web + shared) → clean
  • Real-DB verification: booted a worktree-stack this session and confirmed it healthy, but the shared Playwright global-setup.ts login step timed out against this instance before my spec ever ran (unrelated to this diff — a fresh-fixtures login flow issue in the stack itself, not something this wave's files touch). Stack torn down cleanly afterward. The e2e spec (manual-network-asset.spec.ts) is unverified against a live stack; recommend a follow-up run before merge if a live check is wanted.

No merge, no issue close — leaving both to Todd.

…aseline +1 for "Website" (#5213)

Test Web failed on two i18n contracts: a dynamic translation key
(ternary inside t()) and the de-DE exact-English duplicate baseline for
discovery.json, where assetTypes.website is legitimately "Website" in
German. Keys are now static per branch; baseline bumped with the reason.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pc21knHQGa6fCM7UA9YtKX
@ToddHebebrand

Copy link
Copy Markdown
Collaborator Author

Test Web red on two i18n contracts (dynamic t() key at AddNetworkAssetModal.tsx:273; de-DE discovery.json exact-English baseline 26→27 because assetTypes.website is legitimately "Website" in German). Orchestrator fix 24f8d3d: static keys per branch + baseline bump with reason. i18n suite 146/146, modal tests green, tsc clean. Enqueuing on green head CI.

@ToddHebebrand
ToddHebebrand added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 6dbded0 Sep 8, 2026
70 checks passed
@ToddHebebrand
ToddHebebrand deleted the feature/5228-manual-network-asset/wave-5231 branch September 8, 2026 08:20
ToddHebebrand pushed a commit that referenced this pull request Sep 8, 2026
#5213's manual network assets W02 (#5258) and W03 (#5260) landed while this
wave was open; both extend the same Devices surfaces. Resolution keeps both
features in full:

- DevicesPage: ONE Add control. main's inline split menu is folded into this
  wave's shared AddAssetMenu component, now three items (Install agent…,
  Add asset manually…, Add network asset…) in both the header and empty-state
  instances. Testids follow main's names (devices-page-add-menu*), so #5260's
  e2e spec is unchanged; this wave's Page Object was renamed to match. Side
  benefit: main's two instances shared one addMenuOpen flag (they opened
  together); AddAssetMenu holds per-instance state.
- Locale catalogs (8): structural 3-way JSON union, plus devicesPage.addAsset /
  installAgent / addManualAssetAction folded into devicesPage.addMenu.* so the
  two features share one namespace.
- DeviceList / columnVisibility: serial + assetTag + location (this wave) AND
  source (#5213) all kept.
- bulkActionGating: took main's dedicated
  deviceActions.unavailable.unknown string over this wave's notOnline reuse —
  a manual asset was never online, so "not online" is the less honest label.
- DeviceList.statusSortRank: kept this wave's ordering (decommissioned 6,
  unknown 7) — "no probe has ever run" is a different axis, not a state worse
  than offline.
- no-silent-mutations: both handlers registered, count 123 -> 125.
- translationCoverage: baselines recomputed as base + both deltas.

Verified: apps/web 828 files / 9336 tests pass; apps/web and apps/api tsc clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016tcUieLXQuupVjBUSZUn5j
ToddHebebrand pushed a commit that referenced this pull request Sep 8, 2026
…ec.ts's final assertion has a cell to read (#5265)

The spec's last step asserts the created row's device-<id>-source cell reads
"manual", but 'source' is an opt-in column (columnVisibility.ts:
DEFAULT_VISIBLE_COLUMNS deliberately excludes it — agent rows have no source,
so default-on would show a column of dashes for the common agent-only fleet).
That gap predates this fix branch: the opt-in decision shipped in W02 (#5258),
two commits before the spec itself was added in W03 (#5260) — so the
assertion was inconsistent with the shipped default from the start.

The Columns menu that toggles it carries no data-testid (out of that wave's
file-ownership scope), so seed the persisted localStorage preference directly
via addInitScript before the first navigation instead of driving that menu
with a non-testid selector. A single-entry columns list still gets every
other column its own catalog default via columnVisibility.ts's merge-on-read,
so this only adds 'source' visibility — no other column is affected.

Verified end-to-end against a worktree-stack: the spec now passes in full,
including the previously-blocked final assertion, confirming the DevicesPage
fix (previous commit) holds under a real browser refresh.

Refs #5265

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RTMowR8XgvbgZakWhtbiDv
pull Bot pushed a commit to hcaldicott/breeze that referenced this pull request Sep 8, 2026
… refresh (LanternOps#5265) (LanternOps#5271)

## Root cause

`AddNetworkAssetModal.onCreated` calls `DevicesPage.refreshDevices()`,
which flips `loading` back to `true`. Every DevicesPage render branch
(org-context-failed, loading, access-denied, error, main) was a
**separate `return`**, so the loading skeleton unmounted
`<AddNetworkAssetModal>` entirely instead of swapping only the content
underneath it — resetting the modal's internal `createdAsset` state to
`null` the instant the post-create HTTP-check hand-off panel
(`data-testid="asset-post-create"`) was supposed to appear.

## Fix

Hoisted `<AddNetworkAssetModal>` into a `Fragment` that wraps **every**
branch, always as the first child at the same tree position, so React
preserves the component instance across the `loading` transition — only
the second child (skeleton / error state / real content) swaps out
underneath it. This is the "move the modal above the early return" shape
from the issue's proposed fix, chosen over restructuring the loading
skeleton into the page body (would have required guarding a much larger
swath of JSX across four branches) and over lifting `createdAsset` into
page state (would have duplicated state the modal already owns correctly
— the bug is only that DevicesPage discards the component, not that the
state model is wrong).

Also fixed `e2e-tests/tests/manual-network-asset.spec.ts`: its final
assertion checks a `device-<id>-source` cell, but `source` is an opt-in
column (`columnVisibility.ts`) that predates this fix branch — the
opt-in decision shipped in W02 (LanternOps#5258), two commits before the spec was
added in W03 (LanternOps#5260), so the assertion was inconsistent with the shipped
default from the start and unrelated to the LanternOps#5265 bug itself. Seeded the
persisted column-visibility preference via `addInitScript` (the Columns
menu that would toggle it in the UI carries no `data-testid`, out of
that wave's file-ownership scope) rather than touch that menu.

## Test evidence

- **New regression test** (`DevicesPage.postCreateHandoff.test.tsx`):
reproduces the exact remount via a stateful fake `AddNetworkAssetModal`
whose internal `created` flag can only survive a `loading` flip if the
real component instance stays mounted. Verified **red** against the
pre-fix `DevicesPage.tsx` (panel disappears), **green** after.
- `apps/web` full suite: **828 files / 9311 tests passed**.
- `npx tsc --noEmit -p .`: clean.
- **E2E** (`e2e-tests/tests/manual-network-asset.spec.ts`) against a
fresh `worktree-stack`: first run confirmed the actual LanternOps#5265 behavior —
hand-off panel appeared and survived the create → refresh flow correctly
— but failed on the unrelated pre-existing `source`-column gap described
above. After the spec fix, re-ran end-to-end: **1 passed**, including
that final assertion.

Closes LanternOps#5265
Refs LanternOps#5228

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01RTMowR8XgvbgZakWhtbiDv

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

W03: Website/URL targets, partner inventory API, docs, e2e

1 participant